Skip to content

Conversation

@rodrigok
Copy link
Member

@rodrigok rodrigok commented Oct 9, 2025

https://rocketchat.atlassian.net/browse/FDR-195

Depends on RocketChat/homeserver#270

Proposed changes (including videos or screenshots)

Issue(s)

Steps to test or reproduce

Further comments

Summary by CodeRabbit

  • New Features

    • Support for end-to-end encrypted federation messages: encrypted payloads can be sent, received, referenced, and stored even without plaintext.
  • Improvements

    • Encrypted messages support edits, quotes/replies, and threading while preserving cross-server context.
    • Federation handlers add validation, error logging, and early no-ops for non-local role/room events.
    • Invite retry/backoff timing and logging adjusted for seconds-based delays.
  • Chores

    • Bumped federation SDK dependency to a newer patch version.

Copilot AI review requested due to automatic review settings October 9, 2025 19:28
@rodrigok rodrigok requested a review from a team as a code owner October 9, 2025 19:28
@dionisio-bot
Copy link
Contributor

dionisio-bot bot commented Oct 9, 2025

Looks like this PR is not ready to merge, because of the following issues:

  • This PR is missing the 'stat: QA assured' label

Please fix the issues and try again

If you have any trouble, please check the PR guidelines

@changeset-bot
Copy link

changeset-bot bot commented Oct 9, 2025

⚠️ No Changeset found

Latest commit: f974305

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This pull request adds support for accepting and storing encrypted messages in the federation system. The changes enable handling of encrypted Matrix messages by introducing new parameters for encryption content and updating the message service interface.

  • Added support for encrypted message content with algorithm and ciphertext fields
  • Implemented encrypted message event handler in the Matrix federation module
  • Updated message service interface to accept optional encrypted content alongside regular messages

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
packages/core-services/src/types/IMessageService.ts Updated interface to support optional encrypted content and made msg parameter optional
ee/packages/federation-matrix/src/events/message.ts Added new event handler for processing encrypted Matrix messages with support for threads, replies, and edits
apps/meteor/server/services/messages/service.ts Implemented encrypted content storage in the message service with proper message type assignment

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment on lines 28 to 30
algorithm: string,
ciphertext: string,
},
Copy link

Copilot AI Oct 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Inconsistent indentation: the properties inside e2e_content should use tabs to match the surrounding code style.

Suggested change
algorithm: string,
ciphertext: string,
},
algorithm: string,
ciphertext: string,
},

Copilot uses AI. Check for mistakes.
Comment on lines 104 to 106
algorithm: string,
ciphertext: string,
},
Copy link

Copilot AI Oct 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Inconsistent indentation: the properties inside e2e_content should use tabs to match the surrounding code style.

Suggested change
algorithm: string,
ciphertext: string,
},
algorithm: string,
ciphertext: string,
},

Copilot uses AI. Check for mistakes.
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 9, 2025

Note

Other AI code review bot(s) detected

CodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review.

Walkthrough

Adds handling for homeserver.matrix.encrypted events: parses ciphertext and m.relates_to (threads/replies/quotes/edits), resolves user/room, updates or saves messages via MessageService.saveMessageFromFederation (now accepts optional msg and optional e2e_content), and includes t: 'e2e'/content in federation payloads when present.

Changes

Cohort / File(s) Summary
Federation: encrypted message handling
ee/packages/federation-matrix/src/events/message.ts
Adds homeserver.matrix.encrypted handler: validates ciphertext, resolves user and room, parses m.relates_to for threads/replies/quotes/edits, updates existing messages or saves new federation messages with e2e_content and federation_event_id, and logs outcomes.
Message service API (types)
packages/core-services/src/types/IMessageService.ts
Updates saveMessageFromFederation parameter type: msg is now optional (msg?: string) and adds e2e_content?: { algorithm: string; ciphertext: string }.
Message service implementation
apps/meteor/server/services/messages/service.ts
Changes saveMessageFromFederation signature to accept optional e2e_content; msg made optional; when e2e_content is provided, outgoing federation payload includes t: 'e2e' and content: e2e_content.
Room event guards
ee/packages/federation-matrix/src/events/room.ts
Adds early-exit guards in room name and room role handlers to return early when the alleged local user is not local.
Dependency bumps
ee/packages/federation-matrix/package.json, packages/core-services/package.json
Bumps @rocket.chat/federation-sdk from 0.1.21 to 0.1.23.
Invite backoff logic
ee/packages/federation-matrix/src/api/_matrix/invite.ts
Adjusts backoff computation from Math.max(625, delaySec ** 2) to Math.min(625, delaySec ** 2) and shifts delay semantics/log message from milliseconds to seconds.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant HS as Matrix Homeserver
  participant FH as Federation Handler
  participant MS as MessageService
  participant DB as Database

  HS->>FH: m.room.encrypted (event with ciphertext)
  FH->>FH: validate ciphertext, resolve user & room
  FH->>FH: parse m.relates_to (thread/reply/quote/edit)

  alt edit of encrypted message
    FH->>DB: find original by federation_event_id
    alt original found & content changed
      FH->>MS: updateMessage(..., e2e_content)
      MS->>DB: update record
      DB-->>MS: OK
      MS-->>FH: OK
    else skip / log
    end
  else new encrypted message
    FH->>MS: saveMessageFromFederation({ fromId, rid, e2e_content, federation_event_id, thread? })
    MS->>DB: insert record (payload includes t: 'e2e' and content)
    DB-->>MS: OK
    MS-->>FH: OK
  end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Suggested labels

stat: ready to merge, stat: QA assured

Suggested reviewers

  • rodrigok
  • ricardogarim

Poem

I hop through encrypted halls tonight,
Cipher tucked close, hidden from sight.
Threads and quotes I stitch and mend,
Saving whispers the homeservers send.
t: 'e2e' snug — carrot key in my bite. 🥕🔐

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Out of Scope Changes Check ⚠️ Warning This pull request includes modifications to room.Name and room.Role event handlers and invite retry backoff logic that do not relate to handling m.room.encrypted federation messages and appear outside the stated objectives of FDR-195. Please remove or move the unrelated changes to separate pull requests and limit this PR to only the additions required for accepting and storing encrypted federation messages.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title clearly and concisely summarizes the main change: supporting encrypted federation messages by accepting and storing them without extraneous details.
Linked Issues Check ✅ Passed The changes introduce a handler for m.room.encrypted events and extend saveMessageFromFederation to accept and persist end-to-end encrypted content, which prevents unsupported event errors and ensures encrypted messages are stored, directly fulfilling the objectives of FDR-195.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat/federation-m.room.enecrypted

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Jira integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 23dcbd8 and f974305.

📒 Files selected for processing (1)
  • ee/packages/federation-matrix/src/api/_matrix/invite.ts (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
ee/packages/federation-matrix/src/api/_matrix/invite.ts (1)
apps/meteor/server/services/omnichannel/queue.ts (1)
  • delay (34-37)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
  • GitHub Check: 🔨 Test Storybook / Test Storybook
  • GitHub Check: 🔨 Test Unit / Unit Tests
  • GitHub Check: 🔎 Code Check / Code Lint
  • GitHub Check: 🔎 Code Check / TypeScript
  • GitHub Check: 📦 Meteor Build - coverage
  • GitHub Check: CodeQL-Build
  • GitHub Check: CodeQL-Build
🔇 Additional comments (1)
ee/packages/federation-matrix/src/api/_matrix/invite.ts (1)

147-148: Critical bug fix: Exponential backoff now works correctly.

The change from Math.max to Math.min fixes a critical logic error where the old code would immediately jump to the maximum delay (625) on the first retry, defeating the purpose of exponential backoff. The new implementation correctly starts at 25 seconds and exponentially increases up to the 625-second cap.

The log message update from "ms" to "s" correctly reflects that the delay is in seconds (consistent with line 151's multiplication by 1000 for setTimeout).

Note: This fix appears unrelated to the PR's stated objective of handling encrypted messages. Consider whether this should be in a separate PR for clearer change tracking.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

♻️ Duplicate comments (2)
packages/core-services/src/types/IMessageService.ts (1)

27-30: Fix indentation to use tabs.

The properties inside e2e_content use spaces for indentation instead of tabs, which is inconsistent with the rest of the file's code style.

Apply this diff to fix the indentation:

 	e2e_content?: {
-            algorithm: string,
-            ciphertext: string,
-        },
+		algorithm: string;
+		ciphertext: string;
+	};

Note: Also changed trailing commas to semicolons to match the surrounding code style.

apps/meteor/server/services/messages/service.ts (1)

103-106: Fix indentation to use tabs.

The properties inside e2e_content use spaces for indentation instead of tabs, which is inconsistent with the rest of the file's code style.

Apply this diff to fix the indentation:

 	e2e_content?: {
-            algorithm: string,
-            ciphertext: string,
-        },
+		algorithm: string;
+		ciphertext: string;
+	};

Note: Also changed trailing commas to semicolons to match the surrounding code style.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Jira integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 652d360 and b0ab2cc.

📒 Files selected for processing (3)
  • apps/meteor/server/services/messages/service.ts (2 hunks)
  • ee/packages/federation-matrix/src/events/message.ts (1 hunks)
  • packages/core-services/src/types/IMessageService.ts (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
ee/packages/federation-matrix/src/events/message.ts (2)
ee/packages/federation-matrix/src/events/room.ts (1)
  • room (8-73)
packages/core-typings/src/IMessage/IMessage.ts (2)
  • isThreadMessage (317-317)
  • isEditedMessage (264-271)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: 📦 Build Packages
  • GitHub Check: CodeQL-Build
  • GitHub Check: CodeQL-Build
🔇 Additional comments (3)
packages/core-services/src/types/IMessageService.ts (1)

12-35: Type definitions correctly support encrypted messages.

The changes appropriately make msg optional and introduce e2e_content to support encrypted federation messages. The interface clearly defines the contract for handling both plaintext and encrypted messages.

apps/meteor/server/services/messages/service.ts (1)

123-126: Encrypted content correctly propagated to federation payload.

The implementation appropriately includes the e2e_content in the message payload with type 'e2e' when encrypted content is provided. The conditional spreading ensures the field is only included when necessary.

ee/packages/federation-matrix/src/events/message.ts (1)

316-329: Verify encrypted quoted message edits preserve quote metadata
The encrypted branch’s updateMessage call (lines 316–329) only carries over algorithm and ciphertext, omitting the quote URL and metadata formatting applied in the plaintext handler. Confirm that encrypted quoted edits reconstruct and persist quote references as the plaintext path does.

Comment on lines +265 to +377
emitter.on('homeserver.matrix.encrypted', async (data) => {
try {
if (!data.content.ciphertext) {
logger.debug('No message content found in event');
return;
}

// at this point we know for sure the user already exists
const user = await Users.findOneByUsername(data.sender);
if (!user) {
throw new Error(`User not found for sender: ${data.sender}`);
}

const room = await Rooms.findOne({ 'federation.mrid': data.room_id });
if (!room) {
throw new Error(`No mapped room found for room_id: ${data.room_id}`);
}

const relation = data.content['m.relates_to'];

// SPEC: For example, an m.thread relationship type denotes that the event is part of a “thread” of messages and should be rendered as such.
const hasRelation = relation && 'rel_type' in relation;

const isThreadMessage = hasRelation && relation.rel_type === 'm.thread';

const threadRootEventId = isThreadMessage && relation.event_id;

// SPEC: Though rich replies form a relationship to another event, they do not use rel_type to create this relationship.
// Instead, a subkey named m.in_reply_to is used to describe the reply’s relationship,
const isRichReply = relation && !('rel_type' in relation) && 'm.in_reply_to' in relation;

const quoteMessageEventId = isRichReply && relation['m.in_reply_to']?.event_id;

const thread = threadRootEventId ? await getThreadMessageId(threadRootEventId) : undefined;

const isEditedMessage = hasRelation && relation.rel_type === 'm.replace';
if (isEditedMessage && relation.event_id) {
logger.debug('Received edited message from Matrix, updating existing message');
const originalMessage = await Messages.findOneByFederationId(relation.event_id);
if (!originalMessage) {
logger.error('Original message not found for edit:', relation.event_id);
return;
}
if (originalMessage.federation?.eventId !== relation.event_id) {
return;
}
if (originalMessage.content?.ciphertext === data.content.ciphertext) {
logger.debug('No changes in message content, skipping update');
return;
}

if (quoteMessageEventId) {
await Message.updateMessage(
{
...originalMessage,
content: {
algorithm: data.content.algorithm,
ciphertext: data.content.ciphertext,
},
},
user,
originalMessage,
);
return;
}

await Message.updateMessage(
{
...originalMessage,
content: {
algorithm: data.content.algorithm,
ciphertext: data.content.ciphertext,
},
},
user,
originalMessage,
);
return;
}

if (quoteMessageEventId) {
const originalMessage = await Messages.findOneByFederationId(quoteMessageEventId);
if (!originalMessage) {
logger.error('Original message not found for quote:', quoteMessageEventId);
return;
}
await Message.saveMessageFromFederation({
fromId: user._id,
rid: room._id,
e2e_content: {
algorithm: data.content.algorithm,
ciphertext: data.content.ciphertext,
},
federation_event_id: data.event_id,
thread,
});
return;
}

await Message.saveMessageFromFederation({
fromId: user._id,
rid: room._id,
e2e_content: {
algorithm: data.content.algorithm,
ciphertext: data.content.ciphertext,
},
federation_event_id: data.event_id,
thread,
});
} catch (error) {
logger.error(error, 'Error processing Matrix message:');
}
});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major

Refactor duplicated logic into shared helper functions.

The encrypted message handler (lines 265-377) duplicates ~110 lines of logic from the plaintext handler (lines 115-263), including user/room resolution, relation parsing, thread handling, and quote detection. This violates DRY principles and increases maintenance burden.

Consider extracting shared logic into helper functions:

  1. Message context resolution (user, room, relations, threads):
async function resolveMessageContext(data: { sender: string; room_id: string; content: any }) {
	const user = await Users.findOneByUsername(data.sender);
	if (!user) {
		throw new Error(`User not found for sender: ${data.sender}`);
	}

	const room = await Rooms.findOne({ 'federation.mrid': data.room_id });
	if (!room) {
		throw new Error(`No mapped room found for room_id: ${data.room_id}`);
	}

	const relation = data.content['m.relates_to'];
	const hasRelation = relation && 'rel_type' in relation;
	const isThreadMessage = hasRelation && relation.rel_type === 'm.thread';
	const threadRootEventId = isThreadMessage && relation.event_id;
	const isRichReply = relation && !('rel_type' in relation) && 'm.in_reply_to' in relation;
	const quoteMessageEventId = isRichReply && relation['m.in_reply_to']?.event_id;
	const thread = threadRootEventId ? await getThreadMessageId(threadRootEventId) : undefined;
	const isEditedMessage = hasRelation && relation.rel_type === 'm.replace';

	return { user, room, relation, thread, quoteMessageEventId, isEditedMessage };
}
  1. Edit handling logic (extract common validation and update flow)

  2. Quote handling logic (extract common quote message resolution)

This would reduce the handlers to ~30-40 lines each, focusing only on content-specific logic (plaintext formatting vs. encrypted content handling).

🤖 Prompt for AI Agents
ee/packages/federation-matrix/src/events/message.ts around lines 115-377: the
encrypted handler (265-377) duplicates the plaintext handler (115-263) — extract
shared responsibilities into helpers: 1) create resolveMessageContext(data) that
finds user and room, parses relation, computes
hasRelation/isEditedMessage/isThreadMessage/threadRootEventId/thread (via
getThreadMessageId) and quoteMessageEventId, and returns { user, room, relation,
thread, quoteMessageEventId, isEditedMessage }; 2) create
handleEdit(originalMessage, data, user) that encapsulates the edit validation
(existence, federation id match, content equality) and performs
Message.updateMessage when needed, returning a boolean indicating it handled the
event; 3) create saveOrQuoteMessage({ userId, roomId, thread,
federation_event_id, e2e_content, quote }) that resolves quoted original message
if quote provided and calls Message.saveMessageFromFederation; then replace the
duplicated blocks in both handlers to call resolveMessageContext, use handleEdit
when isEditedMessage, and use saveOrQuoteMessage for quote/plain saves, keeping
original logging and try/catch semantics.

Comment on lines +267 to +270
if (!data.content.ciphertext) {
logger.debug('No message content found in event');
return;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Validate both algorithm and ciphertext fields.

The handler only validates ciphertext presence but not algorithm. Both fields are required for encrypted content according to the type definition in IMessageService.ts.

Apply this diff to validate both fields:

-		if (!data.content.ciphertext) {
-			logger.debug('No message content found in event');
+		if (!data.content.ciphertext || !data.content.algorithm) {
+			logger.debug('Missing required encrypted content fields (algorithm or ciphertext)');
 			return;
 		}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (!data.content.ciphertext) {
logger.debug('No message content found in event');
return;
}
if (!data.content.ciphertext || !data.content.algorithm) {
logger.debug('Missing required encrypted content fields (algorithm or ciphertext)');
return;
}
🤖 Prompt for AI Agents
In ee/packages/federation-matrix/src/events/message.ts around lines 267 to 270,
the handler only checks for data.content.ciphertext but must validate both
data.content.ciphertext and data.content.algorithm; update the conditional to
verify both fields are present, log a clear debug message when either is missing
(e.g., "Missing encrypted message fields: algorithm or ciphertext") and return
early, ensuring encrypted content processing only proceeds when both algorithm
and ciphertext exist.

Comment on lines +300 to +314
const isEditedMessage = hasRelation && relation.rel_type === 'm.replace';
if (isEditedMessage && relation.event_id) {
logger.debug('Received edited message from Matrix, updating existing message');
const originalMessage = await Messages.findOneByFederationId(relation.event_id);
if (!originalMessage) {
logger.error('Original message not found for edit:', relation.event_id);
return;
}
if (originalMessage.federation?.eventId !== relation.event_id) {
return;
}
if (originalMessage.content?.ciphertext === data.content.ciphertext) {
logger.debug('No changes in message content, skipping update');
return;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major

🧩 Analysis chain

Verify edit handling for encrypted messages matches Matrix spec.

The encrypted message edit handler doesn't check for m.new_content (line 301), unlike the plaintext handler (line 155). For encrypted messages, verify whether edited content is provided in data.content['m.new_content'] or directly in data.content.

If the Matrix specification requires m.new_content for encrypted message edits as well, apply this diff:

-		if (isEditedMessage && relation.event_id) {
+		if (isEditedMessage && relation.event_id && data.content['m.new_content']) {
 			logger.debug('Received edited message from Matrix, updating existing message');

Additionally, the content comparison (line 311) should potentially compare against m.new_content:

-		if (originalMessage.content?.ciphertext === data.content.ciphertext) {
+		if (originalMessage.content?.ciphertext === data.content['m.new_content']?.ciphertext) {
 			logger.debug('No changes in message content, skipping update');

🌐 Web query:

Does the Matrix specification require m.new_content for encrypted message edits (m.room.encrypted with m.replace relation)?

💡 Result:

Short answer: Yes — the spec requires the replacement (edit) to include m.new_content, and for encrypted edits that m.new_content be inside the decrypted payload of the m.room.encrypted event. [1][2]

Notes/refs:

  • MSC2676 / message-editing: "The content of a message edit event must contain a m.new_content property..." and for encrypted replacements "m.new_content is placed in the content of the encrypted payload." [1]
  • The Client‑Server API / related spec discussion and issues note the practical/server-side complications (servers can't read encrypted payloads), but that is an implementation/aggregation concern — it does not remove the requirement that decrypted edit payloads contain m.new_content. [2][3]

Require m.new_content for encrypted edits

Apply the following changes in ee/packages/federation-matrix/src/events/message.ts (around lines 300–314):

-   if (isEditedMessage && relation.event_id) {
+   if (isEditedMessage && relation.event_id && data.content['m.new_content']) {
        logger.debug('Received edited message from Matrix, updating existing message');
...
-   if (originalMessage.content?.ciphertext === data.content.ciphertext) {
+   if (originalMessage.content?.ciphertext === data.content['m.new_content']?.ciphertext) {
        logger.debug('No changes in message content, skipping update');
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const isEditedMessage = hasRelation && relation.rel_type === 'm.replace';
if (isEditedMessage && relation.event_id) {
logger.debug('Received edited message from Matrix, updating existing message');
const originalMessage = await Messages.findOneByFederationId(relation.event_id);
if (!originalMessage) {
logger.error('Original message not found for edit:', relation.event_id);
return;
}
if (originalMessage.federation?.eventId !== relation.event_id) {
return;
}
if (originalMessage.content?.ciphertext === data.content.ciphertext) {
logger.debug('No changes in message content, skipping update');
return;
}
const isEditedMessage = hasRelation && relation.rel_type === 'm.replace';
if (isEditedMessage && relation.event_id && data.content['m.new_content']) {
logger.debug('Received edited message from Matrix, updating existing message');
const originalMessage = await Messages.findOneByFederationId(relation.event_id);
if (!originalMessage) {
logger.error('Original message not found for edit:', relation.event_id);
return;
}
if (originalMessage.federation?.eventId !== relation.event_id) {
return;
}
if (originalMessage.content?.ciphertext === data.content['m.new_content']?.ciphertext) {
logger.debug('No changes in message content, skipping update');
return;
}

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

♻️ Duplicate comments (1)
packages/core-services/package.json (1)

38-38: Version bump matches federation-matrix package.

This dependency update to 0.1.23 maintains consistency with the change in ee/packages/federation-matrix/package.json. Verification of the version is covered in the previous review comment.

🧹 Nitpick comments (1)
ee/packages/federation-matrix/src/events/room.ts (1)

57-59: Early return for non-local users improves robustness.

The guard correctly prevents processing role changes for non-local users, avoiding the "mapped user not found" error that would have occurred at line 61. This is a sensible improvement to the homeserver.matrix.room.role handler.

Optionally, consider adding a brief comment to document the intent:

+	// Skip role processing for non-local users
 	if (!allegedUserLocalIsLocal) {
 		return;
 	}
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Jira integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between b0ab2cc and 4c37f1a.

⛔ Files ignored due to path filters (1)
  • yarn.lock is excluded by !**/yarn.lock, !**/*.lock
📒 Files selected for processing (4)
  • apps/meteor/server/services/messages/service.ts (2 hunks)
  • ee/packages/federation-matrix/package.json (1 hunks)
  • ee/packages/federation-matrix/src/events/room.ts (1 hunks)
  • packages/core-services/package.json (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • apps/meteor/server/services/messages/service.ts

"@rocket.chat/core-typings": "workspace:^",
"@rocket.chat/emitter": "^0.31.25",
"@rocket.chat/federation-sdk": "0.1.21",
"@rocket.chat/federation-sdk": "0.1.23",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Verify the federation SDK version and check for security advisories.

The version bump from 0.1.21 to 0.1.23 aligns with the PR's dependency on homeserver PR #270. Ensure this version exists and has no known vulnerabilities.

Run the following script to verify the package version:


🏁 Script executed:

#!/bin/bash
# Description: Verify the @rocket.chat/federation-sdk version exists and check for security advisories

# Check if version 0.1.23 exists on npm
npm view @rocket.chat/[email protected] version 2>/dev/null

if [ $? -eq 0 ]; then
  echo "✓ Version 0.1.23 exists"
else
  echo "✗ Version 0.1.23 not found on npm"
fi

# Check for security advisories using GitHub CLI
gh api graphql -f query='
{
  securityVulnerabilities(first: 5, ecosystem: NPM, package: "@rocket.chat/federation-sdk") {
    nodes {
      advisory {
        summary
        severity
        publishedAt
      }
      vulnerableVersionRange
      firstPatchedVersion {
        identifier
      }
    }
  }
}'

Length of output: 83


Fix the missing federation-sdk version reference
ee/packages/federation-matrix/package.json references "@rocket.chat/federation-sdk": "0.1.23", but version 0.1.23 isn’t published on npm. Publish v0.1.23 or revert to a published version.

🤖 Prompt for AI Agents
In ee/packages/federation-matrix/package.json around line 41, the dependency
entry references "@rocket.chat/federation-sdk": "0.1.23", which is not published
to npm; update the package.json to point to a published version (e.g., revert to
the latest published tag) or ensure v0.1.23 is published, then run npm install /
yarn to validate; update any lockfiles and CI references accordingly.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
ee/packages/federation-matrix/src/events/room.ts (1)

9-23: Add early-exit guard for non-local users in the homeserver.matrix.room.name handler
Before looking up the user, call

const [allegedUsernameLocal,, allegedUserLocalIsLocal] = getUsernameServername(userId, services.config.serverName);
if (!allegedUserLocalIsLocal) {
  return;
}

then proceed with Users.findOneByUsername(allegedUsernameLocal, …). This matches the room.role handler and prevents errors when remote users send name events.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Jira integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between b0ab2cc and 4c37f1a.

⛔ Files ignored due to path filters (1)
  • yarn.lock is excluded by !**/yarn.lock, !**/*.lock
📒 Files selected for processing (4)
  • apps/meteor/server/services/messages/service.ts (2 hunks)
  • ee/packages/federation-matrix/package.json (1 hunks)
  • ee/packages/federation-matrix/src/events/room.ts (1 hunks)
  • packages/core-services/package.json (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • apps/meteor/server/services/messages/service.ts
🔇 Additional comments (1)
packages/core-services/package.json (1)

38-38: Dependency bump is consistent with federation-matrix package.

The @rocket.chat/federation-sdk upgrade to 0.1.23 mirrors the change in ee/packages/federation-matrix/package.json. Please refer to the verification comment on that file to ensure version validity and security.

"@rocket.chat/core-typings": "workspace:^",
"@rocket.chat/emitter": "^0.31.25",
"@rocket.chat/federation-sdk": "0.1.21",
"@rocket.chat/federation-sdk": "0.1.23",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Verify the SDK version for validity and security.

Ensure the upgraded @rocket.chat/federation-sdk version 0.1.23 is valid and free from security vulnerabilities. This dependency bump is coordinated with packages/core-services/package.json to enable encrypted message handling.


🏁 Script executed:

#!/bin/bash
# Description: Verify the federation-sdk version and check for security advisories

# Check npm registry for version existence
echo "Checking if @rocket.chat/[email protected] exists..."
npm view @rocket.chat/[email protected] version 2>/dev/null || echo "Version not found or registry unavailable"

# Check for security advisories using GitHub CLI
echo -e "\nChecking for security advisories..."
gh api graphql -f query='
{
  securityVulnerabilities(first: 10, ecosystem: NPM, package: "@rocket.chat/federation-sdk") {
    nodes {
      advisory {
        summary
        severity
        publishedAt
      }
      vulnerableVersionRange
      firstPatchedVersion {
        identifier
      }
    }
  }
}'

Length of output: 185


Confirm @rocket.chat/[email protected] is published
The npm lookup failed—0.1.23 isn’t available. Publish the release or revert to a published version before merging.

🤖 Prompt for AI Agents
In ee/packages/federation-matrix/package.json around line 41, the dependency
entry for "@rocket.chat/federation-sdk": "0.1.23" points to a version that isn’t
published; verify publication (e.g., npm view
@rocket.chat/[email protected]) and either publish 0.1.23 or change the
package.json to a known-published version (e.g., the last released version),
then update lockfiles (npm/yarn) and run install to ensure resolution before
merging.

@ggazzo ggazzo force-pushed the feat/federation-m.room.enecrypted branch from 4c37f1a to 50d1471 Compare October 9, 2025 21:13
@codecov
Copy link

codecov bot commented Oct 9, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 66.37%. Comparing base (652d360) to head (f974305).
⚠️ Report is 4 commits behind head on release-7.11.0.

Additional details and impacted files

Impacted file tree graph

@@               Coverage Diff                @@
##           release-7.11.0   #37191    +/-   ##
================================================
  Coverage           66.36%   66.37%            
================================================
  Files                3386     3386            
  Lines              115659   115659            
  Branches            21507    21364   -143     
================================================
+ Hits                76755    76766    +11     
+ Misses              36295    36288     -7     
+ Partials             2609     2605     -4     
Flag Coverage Δ
e2e 57.27% <ø> (+0.01%) ⬆️
unit 71.23% <ø> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@ggazzo ggazzo force-pushed the feat/federation-m.room.enecrypted branch from 50d1471 to 23dcbd8 Compare October 9, 2025 21:30
@ggazzo ggazzo added this to the 7.11.0 milestone Oct 9, 2025
@rodrigok rodrigok merged commit d5b1acf into release-7.11.0 Oct 10, 2025
50 checks passed
@rodrigok rodrigok deleted the feat/federation-m.room.enecrypted branch October 10, 2025 00:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants